check for active MSses before starting DB upgrade #12140
Draft
+59
−24
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov Report✅ All modified and coverable lines are covered by tests.
Additional details and impacted files@@ Coverage Diff @@
## 4.20 #12140 +/- ##
=============================================
- Coverage 16.18% 4.00% -12.18%
=============================================
Files 5657 402 -5255
Lines 498470 32665 -465805
Branches 60493 5808 -54685
=============================================
- Hits 80660 1309 -79351
+ Misses 408830 31203 -377627
+ Partials 8980 153 -8827
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds a check to prevent database upgrades from running when multiple management servers are active. The implementation verifies that only one management server with status 'UP' exists in the database before proceeding with the upgrade process, addressing issue #11973.
- Extracts the upgrade logic into a new
doUpgrades()method for better code organization - Introduces
checkIfStandalone()to query the management_server table and enforce single-server mode - Throws an exception if multiple active management servers are detected, preventing concurrent upgrade attempts
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
engine/schema/src/main/java/com/cloud/upgrade/DatabaseUpgradeChecker.java
Show resolved
Hide resolved
engine/schema/src/main/java/com/cloud/upgrade/DatabaseUpgradeChecker.java
Outdated
Show resolved
Hide resolved
engine/schema/src/main/java/com/cloud/upgrade/DatabaseUpgradeChecker.java
Show resolved
Hide resolved
engine/schema/src/main/java/com/cloud/upgrade/DatabaseUpgradeChecker.java
Outdated
Show resolved
Hide resolved
engine/schema/src/main/java/com/cloud/upgrade/DatabaseUpgradeChecker.java
Show resolved
Hide resolved
Comment on lines
461
to
486
| private void checkIfStandalone() throws CloudRuntimeException { | ||
| boolean standalone = Transaction.execute(new TransactionCallback<>() { | ||
| @Override | ||
| public Boolean doInTransaction(TransactionStatus status) { | ||
| String sql = "SELECT COUNT(*) FROM `cloud`.`management_server` WHERE `status` = 'UP'"; | ||
| try (Connection conn = TransactionLegacy.getStandaloneConnection(); | ||
| PreparedStatement pstmt = conn.prepareStatement(sql); | ||
| ResultSet rs = pstmt.executeQuery()) { | ||
| if (rs.next()) { | ||
| int count = rs.getInt(1); | ||
| return count <= 1; | ||
| } | ||
| } catch (SQLException e) { | ||
| String errorMessage = "Unable to check if the management server is running in standalone mode."; | ||
| LOGGER.error(errorMessage, e); | ||
| throw new CloudRuntimeException(errorMessage, e); | ||
| } | ||
| return true; | ||
| } | ||
| }); | ||
| if (! standalone) { | ||
| String msg = "CloudStack is running multiple management servers and attempting to upgrade. Upgrades can only be run in standalone mode. Skipping database upgrade check."; | ||
| LOGGER.info(msg); | ||
| throw new CloudRuntimeException(msg); | ||
| } | ||
| } |
There was a problem hiding this comment.
The new checkIfStandalone() method lacks test coverage. Consider adding unit tests to verify:
- The behavior when no management servers are running (count = 0)
- The behavior when exactly one management server is running (count = 1)
- The behavior when multiple management servers are running (count > 1)
- The behavior when a SQLException occurs during the check
This is particularly important since this check prevents upgrades from proceeding when multiple servers are detected.
engine/schema/src/main/java/com/cloud/upgrade/DatabaseUpgradeChecker.java
Outdated
Show resolved
Hide resolved
engine/schema/src/main/java/com/cloud/upgrade/DatabaseUpgradeChecker.java
Show resolved
Hide resolved
engine/schema/src/main/java/com/cloud/upgrade/DatabaseUpgradeChecker.java
Show resolved
Hide resolved
engine/schema/src/main/java/com/cloud/upgrade/DatabaseUpgradeChecker.java
Outdated
Show resolved
Hide resolved
engine/schema/src/main/java/com/cloud/upgrade/DatabaseUpgradeChecker.java
Outdated
Show resolved
Hide resolved
engine/schema/src/main/java/com/cloud/upgrade/DatabaseUpgradeChecker.java
Outdated
Show resolved
Hide resolved
engine/schema/src/main/java/com/cloud/upgrade/DatabaseUpgradeChecker.java
Outdated
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
engine/schema/src/main/java/com/cloud/upgrade/DatabaseUpgradeChecker.java
Outdated
Show resolved
Hide resolved
Co-authored-by: Copilot <[email protected]>
|
Packaging result [SF]: ✖️ el8 ✖️ el9 ✖️ debian ✖️ suse15. SL-JID 15845 |
DaanHoogland
commented
Nov 28, 2025
| LOGGER.error(errorMessage, e); | ||
| throw new CloudRuntimeException(errorMessage, e); | ||
| } | ||
| return true; |
Contributor
Author
There was a problem hiding this comment.
maybe be save and return false as we are not sure.
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✖️ debian ✔️ suse15. SL-JID 15861 |
|
[SF] Trillian test result (tid-14888)
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PR...
Fixes: #11973
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Bug Severity
Screenshots (if appropriate):
How Has This Been Tested?
How did you try to break this feature and the system with this change?